feat: adding start and end block filters to Deposits api #560
Conversation
AlexanderLiteplo
requested review from
NikolasHaimerl,
amateima,
ashwinrava,
dohaki,
melisaguevara,
mrice32 and
nicholaspai
as code owners
amateima
left a comment
amateima
left a comment
There was a problem hiding this comment.
Can you share more details how these query params will be used? How big will be the typical block interval? What's the expected calls frequency?
Also, given the size of the interrogated tables, I think new indexes are necessary in those 3 tables where we want to filter by block number
|
Hey @amateima For the block intervals DefiLlama uses the following: // each chain should have number of blocks per approx. 1.5 or 2 hours. default is 400.
export const maxBlocksToQueryByChain = {
default: 300,
ethereum: 300,
polygon: 1000,
fantom: 800,
arbitrum: 3000,
era: 5000,
linea: 3000,
manta: 800,
blast: 2000,
avax: 3000,
bsc: 2000,
optimism: 3000,
xdai: 400,
aurora: 5400,
celo: 1200,
klaytn: 6000,
sui: 2400, // sui creates a checkpoint about every 3 seconds
solana: 6000,
taiko: 100,
sonic: 10000,
base: 3000,
arbitrum_nova: 3000,
polygon_zkevm: 2000,
scroll: 2000,
mode: 2000,
mantle: 2000,
"b2-mainnet": 2000,
berachain: 3000,
hyperliquid: 3000,
} as { [chain: string]: number };Frequency: Indexes: |
| ]) | ||
| @Index("IX_depositForBurn_finalised", ["finalised"]) | ||
| @Index("IX_depositForBurn_deletedAt", ["deletedAt"]) | ||
| @Index("IX_depositForBurn_chainId_blockNumber", ["chainId", "blockNumber"]) |
There was a problem hiding this comment.
Will the blockNumber query parameters always be used alongside chainId?
Keep in mind that with this IX, blockNumber index won't be used by postgres if chainId is missing from the request parameters.
Also, I think this index is redundant and doesn't bring any performance improvements because we already have UK_depositForBurn_chain_block_txn_log index. The first 2 columns from there are identical to your columns.
There was a problem hiding this comment.
Given the conversation we had, we can remove this index
melisaguevara
left a comment
melisaguevara
left a comment
There was a problem hiding this comment.
looks good! It will be nice to check Alex's comment regarding the index on DepositForBurn and remove if it's indeed not needed before merging.
| @Index("IX_v3FundsDeposited_originChainId_blockNumber", [ | ||
| "originChainId", | ||
| "blockNumber", | ||
| ]) |
There was a problem hiding this comment.
I am not 100% sure but I think we can remove this because we have the IX_deposits_block_chain_logIndex index
|
|
||
| export const DepositForBurnFilledRelayFields = [ | ||
| `NULL::varchar as "relayer"`, | ||
| `"mintAndWithdraw"."blockNumber"::integer as "fillBlockNumber"`, |
There was a problem hiding this comment.
Is it possible we might get into some trouble with the naming here?
fillBlockNumber is not really a fill, but a mint, no?
Need to add these in order to update our defi llama dashboard data source.
This requests in this file:
https://github.com/DefiLlama/bridges-server/blob/master/src/adapters/across/index.ts
will be replaced with simply reading and transforming the data from this api.
In order to support the DefiLlama adapter we also need to return the FIllBlockNumber so I have added that in this PR as well.